SWIFT-749 use ELG provider, update shutdown API #446
Conversation
|
also all this definitely necessitates updating guides/examples but I figured I'd see what you all have to say on the API before I do that |
patrickfreed
left a comment
patrickfreed
left a comment
There was a problem hiding this comment.
API looks good, great job on researching what the current standards are. I just a have a few comments about the internals.
kmahar
force-pushed
the
SWIFT-749/elg-provider
branch
from
46e46e8 to
8434e91
Compare
Codecov Report
@@ Coverage Diff @@
## master #446 +/- ##
==========================================
+ Coverage 76.59% 76.72% +0.12%
==========================================
Files 117 117
Lines 12984 13056 +72
==========================================
+ Hits 9945 10017 +72
Misses 3039 3039
Continue to review full report at Codecov.
|
|
alright I've made significant changes based on our conversations, will make some comments inline. the gist is that the client now only tracks if it has been closed, which is just for the purpose of asserting in |
nbbeeken
left a comment
nbbeeken
left a comment
There was a problem hiding this comment.
Just some nits that are optional so lgtm 🚀
This PR updates our client initializer to accept an
EventLoopGroupProviderrather than anEventLoopGroup.As a result I needed to update our shutdown API since it can no longer return a future.
I took a lot of inspiration writing shutdown code from:
Comments inline